See #110

Oh gosh, sorry, I didn't see that PR. I don't mind if you want to close this, but I'd argue that while AWS doesn't recommend hashes, they do support it. Plus at least two people have run into this in the wild and had the motivation to open a PR to fix it, so it's clearly a real-world issue.

Did you ever look into using the AWS parser? If you did, then we could at least bug AWS support instead of you 😆

You make a good point. I'm willing to reconsider this. Did you see how Laravel's storage facade handles it? It might be at least worth making sure we're taking the same approach.

Digging into how Laravel handles it, they use the Flysystem library, and Flysystem stores the S3 bucket and path separately, sidestepping the issue of deconstructing paths.

Looking at the ZipStream code again, I realize calculateFilesize() is the only place we actually need to deconstruct the URI, so if we can avoid using headObject(['bucket' => ..., 'key' => ...]), we can fix the issue. When I have time later I'll try using the stream wrapper like the other functions already do -- the docs say it supports filesize().

Hmm. filesize($this->getReadableStream()) would be interesting to test out. That already sets up the stream wrapper.

Keep in mind there are two loops over our entire queue of files: first to calculate the sizes and send out a Content-Length header, and then a second time when reading in the file contents. I'm always paranoid of the performance of that first loop. I don't know if setting up the stream wrapper times potentially a ton of files just to get filesize will be any different than a direct headObject call, but it's something we'll want to test. I know we zip thousands of files at once, sometimes.

Sorry for not reporting back earlier, but I've been busy. But I have now investigated using a stream and made an interesting discovery: the AWS stream wrapper already gets the file size every time a new read stream is created, so using the stream to get the filesize is essentially free!

I setup a benchmark on my local that creates 1,000 small 100 character random text files on S3, then zips them to a local file and times it. Running it a couple times gets:

Using $this->getS3Client()->headObject()

Average    : ~250ms
Peak Memory: ~72 MB

Using $this->getReadableStream()->getSize()

Average    : ~190ms
Peak Memory: ~72 MB